Skip to content

fix(plugin-auth): the auth catch-all yields paths better-auth does not own (#4088) - #4092

Merged
os-zhuang merged 1 commit into
mainfrom
claude/hono-static-discovery-hardcoded-2ysa9f
Jul 30, 2026
Merged

fix(plugin-auth): the auth catch-all yields paths better-auth does not own (#4088)#4092
os-zhuang merged 1 commit into
mainfrom
claude/hono-static-discovery-hardcoded-2ysa9f

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Closes #4088#4073 / #4079 实施中发现的加载顺序隐患。

问题

plugin-auth 在自己的 kernel:ready hook 里挂一个覆盖整个 auth 命名空间的 catch-all(/api/v1/auth/*),而它是终结式的 —— 无条件返回 better-auth 的响应,包括 better-auth 不认识该路径时的 404。于是别的插件挂在这个前缀下的路由,只有恰好注册得更早才能被访问到(Hono 对同一路径按注册顺序执行 handler,先返回 Response 的赢)。

这让一个承重面完全取决于 kernel.use() 顺序:

顺序 /auth/me/permissions
HonoServerPlugin 先(今天 os serve 正常应答
AuthPlugin 先 静默 404

而 objectui console 的整个权限层/auth/me/permissionscore/security/auth-gate.ts/me/localization 列为"被门禁拦住的用户必须仍能访问"。这与 #2567#4018 是同一类:不变量靠加载顺序侥幸成立而非被强制。更一般地,任何插件想在 /api/v1/auth/* 下挂路由今天都会被吞掉。

修法

better-auth 返回 404 即视为"这条路径不是我的",落穿给其他匹配者 —— 与注册顺序无关。刻意收窄:

  • 只对 404 落穿。401/403 是 better-auth 的真实回答,不是所有权声明的放弃。
  • 优先级仍归命名空间 owner。better-auth 实现了的路径照旧先赢,只有它不要的才让出去。
  • 无人认领时线上形状不变:逐字返回 better-auth 原本那个 404,而不是 Hono 的 404 Not Found

两处机制细节值得记下,都是踩出来的(已订正到 issue,那里原本的建议补丁两条都写错了):

  1. c.finalized 不能用作判据。 链末尾无人匹配时 Hono 会运行 notFound handler,它设置响应并把该标志翻真,因此分不清"有人接手"与"没人接手"。改用状态码:下游非 404 才算有人接手。代价是下游自己那份 404 body 会被 better-auth 的取代(status 相同),而这个前缀下今天没有任何路由回 404。
  2. 必须 c.res = response,不能 return response ADR-0069 D5 的 IP 门禁在上方注册了 rawApp.use('${basePath}/*'),链里因此有中间件;Hono 的 compose 只在 finalized 为 false 时才把 handler 的返回值赋给 c.res,而 notFound 已把它翻真 —— return 会被静默丢弃,调用方拿到 Hono 的 404 Not Found 文本。

验证

新增 auth-catchall-fallthrough.test.ts(6 例):打真实 Hono app、better-auth 用一张 path → Response 表替身(这样"哪些路径属于 owner"完全可控),并且 catch-all 先注册、具体路由后注册 —— 正是过去会失败的顺序。

已验证会咬git stash 撤掉修正后重跑,/auth/me/permissions/auth/me/localization 两条立刻失败(拿到 better-auth 的 404);其余 4 条是不变量保护(owner 路径不被劫持、401 不落穿、无人认领时 404 逐字保留、每请求只转发一次),两种情况下都应通过。

为此给 plugin-auth 加了 hono devDependency —— 这个测试的价值就在于打真实 Hono 链,换成手写替身等于测假货。

范围 结果
plugin-auth 全量 579 passed(26 文件,含新增 6)
tsc --noEmit + eslint clean

client / plugin-hono-server / http-conformance / rest-auth-gate / dogfood OIDC 授权码流程本地仍在跑(spec 的 DTS 构建是瓶颈),结果我会补在评论里;CI 也覆盖这些。

顺带核实:这个形状没有第二处落在 os serve 路径上


Generated by Claude Code

…t own (#4088)

`registerAuthRoutes` mounts `rawApp.all('${basePath}/*')` over the whole auth
namespace (`/api/v1/auth` by default), and that handler was TERMINAL: it returned
better-auth's response unconditionally, including the 404 better-auth produces
for a path it does not implement. Any other plugin's route under that prefix was
therefore reachable only if it happened to register FIRST — Hono runs handlers
matching a path in registration order and the first to return a Response wins.

That put a load-bearing surface at the mercy of `kernel.use()` order.
`plugin-hono-server` mounts `/auth/me/permissions` and `/auth/me/localization`
from its own `kernel:ready` hook (unconditionally since #4079); objectui's entire
permission layer reads the former and `core/security/auth-gate.ts` allow-lists
the latter as an endpoint a gated user MUST still reach to bootstrap the
remediation UI. Register AuthPlugin before HonoServerPlugin and all of it
silently 404s. Same class as #2567 and #4018: an invariant held by ordering luck
rather than enforced.

A 404 from better-auth now means "this path is not mine" and the catch-all yields,
in either registration order. Deliberately narrow: only 404 falls through (401/403
are real answers, not disclaimers of ownership); precedence still favours the
namespace owner, so better-auth wins every path it implements and only its
leftovers are up for grabs; and when nothing downstream answers, better-auth's own
404 is returned verbatim so the unclaimed-path wire shape is unchanged.

Two mechanics worth recording, both found the hard way:

  * `c.finalized` cannot discriminate "someone answered" from "nobody did" —
    reaching the end of the chain runs Hono's notFound handler, which sets a
    response and flips the flag. Status can: a non-404 downstream means someone
    answered. The trade is a downstream route's own 404 BODY (better-auth's is
    returned instead); status is identical and no route under this prefix answers
    404 today.
  * The fall-back must ASSIGN `c.res`, not `return` the Response. The ADR-0069 D5
    IP gate above registers `rawApp.use('${basePath}/*')`, so this chain contains
    middleware, and Hono's compose only assigns a handler's returned Response
    while `finalized` is false — a `return` here is silently dropped and the
    caller gets Hono's `404 Not Found` text instead of better-auth's JSON.

Guard test drives a real Hono app with better-auth stubbed by a path table,
registering the catch-all FIRST and the specific route SECOND — exactly the order
that used to fail. Verified to bite: reverting the fix fails the two
`/auth/me/*` cases. `hono` added as a plugin-auth devDependency so the test
exercises the shipped handler rather than a stand-in.

Checked for other instances of the shape: `runtime/dispatcher-plugin.ts`
deliberately does NOT register an `/auth/*` wildcard, and the `/api/v1/*`
catch-all in `adapters/hono` has no in-tree dependents. Cloud's `AuthProxyPlugin`
may share the hazard but is out of this repo.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013VZLsKypjrGhiLpio2uWKu
@vercel

vercel Bot commented Jul 30, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Jul 30, 2026 9:22am

Request Review

@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation dependencies Pull requests that update a dependency file tests tooling labels Jul 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth.

9 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/deployment/cli.mdx (via @objectstack/plugin-auth)
  • content/docs/deployment/production-readiness.mdx (via @objectstack/plugin-auth)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/plugin-auth)
  • content/docs/permissions/authentication.mdx (via @objectstack/plugin-auth)
  • content/docs/permissions/sso.mdx (via @objectstack/plugin-auth)
  • content/docs/plugins/index.mdx (via @objectstack/plugin-auth)
  • content/docs/plugins/packages.mdx (via @objectstack/plugin-auth)
  • content/docs/releases/implementation-status.mdx (via @objectstack/plugin-auth)
  • content/docs/releases/v9.mdx (via @objectstack/plugin-auth)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

Copy link
Copy Markdown
Contributor Author

补上正文里说会贴的跨包结果 —— 本地全绿:

范围 结果
qa/dogfood OIDC 授权码流程 3 passed
plugin-auth 全量 579 passed(26 文件,含新增 6)
client 全量 200 passed
plugin-hono-server 全量 132 passed
qa/http-conformance 全量 46 passed
rest auth-gate 4 passed
tsc --noEmit + eslint clean

dogfood 那条是这次最实的验证:它驱动一遍真实的 OIDC 授权码流程,也就是真的走 better-auth 自己的端点、经过打了补丁的 catch-all。落穿只在 404 上发生,所以 owner 路径这条主干不受影响 —— 这条测试通过即为证。


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review July 30, 2026 09:40
@os-zhuang
os-zhuang merged commit ea24593 into main Jul 30, 2026
18 checks passed
@os-zhuang
os-zhuang deleted the claude/hono-static-discovery-hardcoded-2ysa9f branch July 30, 2026 09:40
os-zhuang added a commit that referenced this pull request Jul 30, 2026
…ire each to be classified (#4116) (#4122)

A handler mounted on `<prefix>/*` claims an entire namespace, and Hono runs every
handler matching a path in registration order with the first Response winning. So
a TERMINAL wildcard makes every other route under that prefix reachable only when
it happens to register first — and plugin registration order is not a contract
anyone declares or tests.

That shape has cost four fixes (#2567, #4018, #4088/#4092, cloud#923) and every
one was found by hand, after the fact, by someone reading the code for an
unrelated reason. The driven tests #4092 and cloud#923 shipped each pin ONE
catch-all and cannot cover the next wildcard someone mounts. What never existed is
the ENUMERATION.

Adds it, following check-route-envelope.mjs (#3843) including the part that
matters most: a site the scan finds but the ledger does not declare is an ERROR,
not a default. Three states — `yields` (verified from the AST, so an entry cannot
rot), `exempt` with a reason, `ratchet` naming the issue. AST rather than regex
because "does this call next()?" is a question about parameter binding: a comment
or string mentioning next(), or an inner closure's own next, all fool a textual
match in the direction that PASSES while the defect ships.

Found 13 namespace-claiming mounts where a manual grep had found 3, and with them
two real, previously untracked instances of the #4088 defect in
packages/adapters/hono — ratcheted under #4117. It also separates that file's
deliberate `${prefix}/*` ownership (ADR-0076 OQ#9) from the two that merely forgot
to yield; in source they look identical.

Verified both directions by hand: reverting #4092's fix fails with the terminal
diagnostic, and a new undeclared catch-all fails as NOT DECLARED.
os-zhuang added a commit that referenced this pull request Jul 30, 2026
…oes not own, and unbreak the #4116 ledger (#4117) (#4133)

Two things, and the second is urgent.

## The fix (#4117)

`app.all(`${prefix}/auth/*`)` claimed a whole namespace and was TERMINAL: it
returned the auth service's response unconditionally, including better-auth's 404
for a path it does not implement, and the legacy `handleAuth` bridge's own
`handled: false` 404. That is #4088's shape. #4116's scan found it; the manual
greps that preceded that scan had not.

A 404 from better-auth, or `handled: false` from the dispatcher, now means "not
this mount's path" and the handler yields. The predicate is the dispatcher's OWN
`handled` flag wherever one exists — an explicit ownership answer beats inferring
one from a status; only the better-auth hand-off lacks such a flag, and there the
404 is the signal, exactly as in #4092.

What this buys here is narrower than #4092 and the code says so: it does NOT make
a later-registered Hono route reachable, because the `${prefix}/*` dispatcher
catch-all below is deliberately terminal (ADR-0076 OQ#9, #3576/#3608) and would
swallow one either way. It means an unowned `/auth/*` path now reaches that gated
`dispatch()` instead of dead-ending in a 404 built one mount earlier, so a domain
handler registered for it becomes reachable — which is this adapter's actual
extension mechanism. The first draft of the tests asserted the #4092 outcome and
failed, which is how the distinction got established rather than assumed.

## Unbreaking main

#4122 shipped the ledger declaring TWO ratcheted mounts here. Between its base
(974c6d4) and its merge, #4087/#4112 landed and DELETED the `/storage/*` bridge —
so the squash produced a main where the ledger declares a mount that no longer
exists, and `check:wildcard-fallthrough` fails on main with "DECLARED but not
found". That is my merge race, and this commit removes the entry.

Worth recording why the guard behaved well here: the stale-entry arm is what
caught it, immediately, on the first run against the new main. And #4112 reached
the same verdict about that wildcard independently, from the other direction —
"the wildcard was wider than the two routes it served". Two independent reads
landing on one defect is the argument for enumerating the shape rather than
finding it by eye each time.

Also extends `callsContinuation`: handing the continuation to a helper
(`yieldUnowned(c, next, …)`) now counts as yielding. Without that the checker
called this fix terminal — a false negative that would have pushed the compose
subtlety toward being duplicated at each call site instead of written once.

Guard: 7 yielding / 0 ratcheted / 5 exempt over 12 mounts. Self-test 17 cases.
adapters/hono 73 passed. Verified both defences bite: reverting the fix fails the
guard with the TERMINAL diagnostic and fails 2 of the 6 new tests. The 3 `tsc`
errors and 2 `eslint` rule-definition errors in this package are pre-existing,
measured identical on a clean tree.


Claude-Session: https://claude.ai/code/session_013VZLsKypjrGhiLpio2uWKu

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plugin-auth 的终结式 catch-all 吞掉 /api/v1/auth/* 下别人的路由 —— console 权限层目前靠 kernel.use() 顺序才活着

2 participants